fix(ItemStatsCalculator): pass itemId to error log - #477
Conversation
WalkthroughThe changes update the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Maple2.Server.Game/Util/ItemStatsCalculator.cs (1)
551-553: Use consistent placeholder name in error log
The log template uses{Item}foritemId; rename it to{ItemId}to improve clarity and consistency.- Log.Logger.Error("Failed to select random item option, no more entries available. ItemId: {Item}", itemId); + Log.Logger.Error("Failed to select random item option, no more entries available. ItemId: {ItemId}", itemId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Maple2.Server.Game/Util/ItemStatsCalculator.cs(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (6)
Maple2.Server.Game/Util/ItemStatsCalculator.cs (6)
116-116: Confirm itemId passed to GetRandomOption
Call site inGetStatscorrectly includesitem.Idas the first argument toGetRandomOption.
161-161: Confirm itemId passed to GetRandomOption in UpdateRandomOption
UpdateRandomOptionnow delegates toGetRandomOption(item.Id, ...)with the new parameter.
395-395: Confirm itemId passed to RandomItemOption in GetStaticOption
The static path inGetStaticOptionnow invokesRandomItemOption(item.Id, ...)matching the updated signature.
416-418: Validate wrapper for RandomItemOption
The newGetRandomOption(int itemId, ...)wrapper correctly forwards to the staticRandomItemOptionmethod.
509-509: Verify updated signature of RandomItemOption
RandomItemOptionnow acceptsint itemIdas its first parameter, satisfying the PR objective to includeitemIdin error logs.
509-553:Details
✅ Verification successful
Ensure all call sites are updated
Please verify that every invocation ofGetRandomOptionandRandomItemOptionacross the codebase uses the new signature.
🏁 Script executed:
#!/bin/bash # Verify updated signatures for RandomItemOption and GetRandomOption rg -nE "RandomItemOption\(|GetRandomOption\(" .Length of output: 123
🏁 Script executed:
#!/bin/bash # Correctly search for all invocations of RandomItemOption and GetRandomOption rg -n -e 'RandomItemOption\(' -e 'GetRandomOption\(' .Length of output: 1145
All calls to RandomItemOption and GetRandomOption have been updated and verified
I’ve searched the entire codebase and found only the following invocations in Maple2.Server.Game/Util/ItemStatsCalculator.cs, all matching the new signatures (using defaults where parameters are omitted):
- Line 116: GetRandomOption(item.Id, itemOption, item.Type, ItemStats.Type.Random)
- Line 161: GetRandomOption(item.Id, itemOption, item.Type, ItemStats.Type.Random, option.Count, presets)
- Line 395: RandomItemOption(item.Id, itemOption, item.Type, statsType)
- Lines 416–417: wrapper definition of GetRandomOption
- Line 509: definition of RandomItemOption
No additional call sites exist.
Summary by CodeRabbit